Enable image popup on mobile device #168#200
Enable image popup on mobile device #168#200jia1 wants to merge 1 commit intonus-cs2103:masterfrom jia1:feat-image-popup
Conversation
jia1
changed the title
|
Ready for review? |
|
Yes. Apologies for leaving out the indication of ready to review. |
|
@acjh can review this one? |
|
|
||
| $(window).load(function() { | ||
| addModalToImg(); | ||
| }); |
There was a problem hiding this comment.
Does this work if executed in $(document).ready as recommended?
| padding: 0; | ||
| } | ||
|
|
||
| .mfp-with-zoom .mfp-container, |
There was a problem hiding this comment.
You didn't initialize popup with zoom options.
|
|
||
| </script> | ||
| <p>-------------------------------------End of document----------------------------------------</p> | ||
| <script type="text/javascript" src="../vendor/magnific-popup/jquery.magnific-popup.min.js"></script> |
There was a problem hiding this comment.
Currently, popups are initialised in common.js, which handbook doesn't load.
But some considerations before you rush to initialise popups in handbook.js:
- Cloning code is generally bad.
- Handbook content now loads in a different manner - you may have to iron those bugs.
- Anyway, there's no image in handbook needing popup, that's not already a good size.
I'd recommend targeting the schedule for now, which is sufficient (and what's intended).
| */ | ||
|
|
||
| /* padding-bottom and top for image */ | ||
| .mfp-no-margins img.mfp-img { |
There was a problem hiding this comment.
.mfp-no-margins is an unused class.
|
@jia1 Note that popup |
3 participants
Fixes #168
Ready for review.
Popped-up images are set to fit the width of the screen.